Skip to content

Add getBulkIdsForIndex to SqlStorage for commcare-core master compat#1765

Merged
dannyroberts merged 3 commits intomasterfrom
dmr/add-getBulkIdsForIndex-to-SqlStorage
Mar 19, 2026
Merged

Add getBulkIdsForIndex to SqlStorage for commcare-core master compat#1765
dannyroberts merged 3 commits intomasterfrom
dmr/add-getBulkIdsForIndex-to-SqlStorage

Conversation

@dannyroberts
Copy link
Member

@dannyroberts dannyroberts commented Mar 17, 2026 •
edited
Loading

Product Description

No user-facing changes. This is an internal compatibility fix.

Technical Summary

commcare-core master added getBulkIdsForIndex(String, Collection<String>) to the IStorageUtilityIndexed interface, but formplayer's SqlStorage never implemented it. This causes a compilation failure when building formplayer against commcare-core master.

Safety Assurance

Safety story

  • The new method follows the identical query pattern as getBulkRecordsForIndex, which is already well-tested in production.
  • Verified that formplayer compiles successfully against commcare-core master with this change.
  • This method is not yet called by any formplayer code — it only satisfies the interface contract.

Automated test coverage

The method is currently only needed for interface compliance. It will be exercised once formplayer fully migrates to commcare-core master.

QA Plan

  • Formplayer compiles against commcare-core master
  • Cross-repo CI passes

Special deploy instructions

  • This PR can be deployed after merge with no further considerations.

Rollback instructions

  • This PR can be reverted after deploy with no further considerations.

Review

  • The set of people pinged as reviewers is appropriate for the level of risk of the change.

🤖 Generated with Claude Code

dannyroberts and others added 2 commits March 17, 2026 17:45
@dannyroberts @claude
Add getBulkIdsForIndex to SqlStorage for commcare-core master compat
627e4b8 
commcare-core master added getBulkIdsForIndex(String, Collection<String>)
to the IStorageUtilityIndexed interface but formplayer's SqlStorage never
implemented it, causing a compilation failure. Implementation follows the
same pattern as the existing getBulkRecordsForIndex method.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@dannyroberts @claude
Update commcare submodule to 5ce1d38 (masterToFormplayer2.60 merge)
1c5edd2 
Picks up getBulkIdsForIndex on the IStorageUtilityIndexed interface
and other master changes needed for cross-repo compatibility.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@codecov
Copy link

codecov bot commented Mar 17, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 0% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.09%. Comparing base (3ee2a65) to head (2c6de08).
⚠️ Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
...va/org/commcare/formplayer/sandbox/SqlStorage.java 0.00% 13 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##             master    #1765      +/-   ##
============================================
- Coverage     70.21%   70.09%   -0.12%     
  Complexity     2029     2029              
============================================
  Files           257      257              
  Lines          7996     8009      +13     
  Branches        761      763       +2     
============================================
  Hits           5614     5614              
- Misses         2099     2112      +13     
  Partials        283      283

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dannyroberts
Copy link
Member Author

dannyroberts commented Mar 17, 2026

I reset the submodule to the last commit on dimagi/commcare-core#1506 to show that tests pass. But that is probably not actually exactly what we want to merge. Better for that PR to first be merged, then for formplayer branch to be merged into master, then for this PR to point to latest master on commcare-core. What I did here was just a quick and dirty way to get something similar to that. (And you can see, tests pass.)

shubham1g5
shubham1g5 approved these changes Mar 18, 2026
View reviewed changes
Copy link
Contributor

@shubham1g5 shubham1g5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method implementation looks right to me here.

@dannyroberts dannyroberts marked this pull request as ready for review March 18, 2026 14:00
@dannyroberts dannyroberts mentioned this pull request Mar 18, 2026
Closed
2 tasks
@dannyroberts dannyroberts merged commit 37c58be into master Mar 19, 2026
5 of 7 checks passed
@dannyroberts dannyroberts deleted the dmr/add-getBulkIdsForIndex-to-SqlStorage branch March 19, 2026 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shubham1g5 shubham1g5 shubham1g5 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dannyroberts @shubham1g5